Fix Apple Container sandbox resource limits - #1215
Conversation
Apple Container sandbox creation ignored configured memory, CPU, and process limits, leaving workloads subject to backend defaults and causing fork failures under low PID ceilings. Pass memory and whole-CPU allocations through the native run flags and map pids_max to Apple Container's nproc ulimit syntax. Reject fractional CPU quotas explicitly because Apple Container cannot represent them without weakening the requested limit.
Greptile SummaryThe PR now passes configured memory, whole-CPU, and process limits to Apple Container and validates unsupported CPU quotas before any container reuse.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| crates/tools/src/sandbox/apple.rs | Applies resource-limit validation before reuse and correctly records policy fingerprints on all successful generated-name paths, resolving both prior findings. |
| crates/tools/src/sandbox/containers.rs | Adds Apple Container memory, whole-CPU, and nproc arguments with explicit CPU validation. |
| crates/tools/src/sandbox/tests/apple.rs | Covers generated-name policy tracking, resource-limit argument construction, and fractional CPU rejection. |
| docs/src/sandbox.md | Documents Apple Container version requirements and whole-CPU granularity. |
Reviews (3): Last reviewed commit: "fix(sandbox): retain policy after name r..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Apple Container reuse could return before validating the configured limits or confirming that the running VM was created with the current policy. Validate resource settings before inspection and associate each reusable container with a fingerprint covering mount and resource policy so changed limits force recreation.
|
@greptile review |
Merging this PR will degrade performance by 10.97%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | session_history_path[default] |
6 µs | 6.7 µs | -10.97% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing almond-football (2cb77aa) with main (af26d7e)
Footnotes
-
9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Record the resource-policy fingerprint only after the current Apple container name is ready. This keeps AlreadyExists generation rotation associated with the successfully created container and avoids destructive recreation on the next readiness check.
|
@greptile review |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Summary
--memoryand--cpuspids_maxwith Apple Container's native--ulimit nproc=<limit>syntaxCloses #1188
Validation
Completed
cargo test -p moltis-tools apple_container_run_argscargo fmt --all -- --checkjust lintcontainer run --rm --cpus 2 --memory 512M ubuntu:25.10 trueRemaining
./scripts/local-validate.sh <PR_NUMBER>pids_maxverification with Apple Container 0.9+; the local CLI is 0.8.0 and predates--ulimitManual QA
backend = "apple-container".memory_limit = "1G",cpu_quota = 2.0, andpids_max = 512under[tools.exec.sandbox.resource_limits].ulimit -uplus a command that forks child processes inside the sandbox.Resource temporarily unavailable.cpu_quota = 0.5and confirm Moltis reports that Apple Container requires a positive whole CPU count.